heuristic: make wildcard character configurable #1418
Conversation
marevers
requested review from
grcevski,
mariomac,
marctc and
rafaelroquetto
as code owners
|
I am not sure if in light of this change the wildCard constant should also be changed to Line 42 in a380cd0 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1418 +/- ##
==========================================
+ Coverage 76.86% 80.93% +4.07%
==========================================
Files 149 149
Lines 15252 15255 +3
==========================================
+ Hits 11723 12347 +624
+ Misses 2911 2301 -610
+ Partials 618 607 -11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
marevers
changed the title
There was a problem hiding this comment.
LGTM! Thanks @marevers again! This looks good to me, I just left one comment around supplying a char to the replace function rather than a string. If we can make it work with char everywhere that would be even better, I just don't know if it's possible.
| @@ -45,7 +45,7 @@ func InitAutoClassifier() error { | |||
| } | |||
|
|
|||
| //nolint:cyclop | |||
| func ClusterPath(path string) string { | |||
| func ClusterPath(path string, replacement string) string { | |||
There was a problem hiding this comment.
Small nit here, can you please change the function second argument to be a char? That way we don't need to dereference the string multiple times.
There was a problem hiding this comment.
I ended up solving this a little differently with my commit 819296b. If I were to use a rune parameter, it would necessitate another type casting to byte as within the ClusterPath function, a byte is required. So I thought it better to use a byte parameter instead and move the deferencing of the string to the ClusterPath call within classifyFromPath. I hope you're okay with that. Tests were adjusted to account for this change.
marevers
force-pushed
the
heuristic-pound-sign
branch
from
0507f11 to
819296b
Compare
|
Looks great @marevers! I think we just need to make the docs linter happy and I'm good to merge this. Great work as usual! |
|
@grcevski I've removed 'will' from lines I changed in the docs that should hopefully now pass the doc linter. I also noticed 'will' is used in many locations in the docs, but I refrained from making any changes anywhere else. |
|
Thanks again for this great work @marevers! |
Closes #1410
Changes the route transform heuristic mode to replace route/path components (gibberish and numeric ids) with a configurable single character.
*remains the default. This improves handling of label selectors, as*is a regex token and can interfere with regex handling in Prometheus/Grafana. By providing the option of changing the replacement character, this can be avoided if required or wanted.